Skip to content

feat(eap): semver-aware ordering via SORT_SEMVER across EAP endpoints - #8103

Merged
phacops merged 33 commits into
masterfrom
claude/semver-sorting-eap-clickhouse-lpg3gs
Jul 21, 2026
Merged

feat(eap): semver-aware ordering via SORT_SEMVER across EAP endpoints#8103
phacops merged 33 commits into
masterfrom
claude/semver-sorting-eap-clickhouse-lpg3gs

Conversation

@phacops

@phacops phacops commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a client opt-in SORT_SEMVER OrderBy option that sorts version-like strings (e.g. sentry.release) by semantic-version precedence instead of lexicographically, fixing bugs like 1.2.10 sorting before 1.2.9. There is no hardcoded attribute list — callers choose the ordering per request via the proto enum, and the sort key is applied to whatever column/attribute they order by.

Ordering bugs fixed:

  • 1.2.10 sorted before 1.2.9 (classic lexicographic bug)
  • 1.2.3-beta.1 sorted after 1.2.3 (a prerelease should come before its stable release)
  • 1.2 and 1.2.0 were not treated as equal (length mismatch)

Approach: a semver_sort_key() helper builds a tuple(arrayResize(arrayMap(x -> toUInt32OrZero(x), splitByChar('.', release_part)), 4), is_stable, raw_string) sort key. It uses only functions available on Altinity 25.3/25.8 — no naturalSortKey (needs upstream 26.3+) and no COLLATE (fails prerelease ordering, needs DangerousRawSQL).

Sort key semantics:

  • Numeric component comparison: 1.2.9 < 1.2.10, 25.3.8.999 < 25.3.8.10041
  • Prerelease sorts before its stable release: 1.2.3-beta.1 < 1.2.3, and PEP 440 dot-dev builds (24.7.0.dev0+<sha>, Sentry's own sentry.release format) < 24.7.0
  • Prerelease of a newer version sorts after an older stable: 1.2.3-beta.1 > 1.2.2
  • Length normalisation: arrayResize pads to 4 components so 1.2 and 1.2.0 share a numeric key ✓
  • package@ prefix stripped: my-pkg@2.0.0 sorts as 2.0.0
  • SemVer build metadata is ignored for precedence: 1.2.3+build sorts as stable 1.2.3
  • Raw-string tiebreaker as the third tuple element gives a deterministic total order over strings that share a numeric key + stability flag, keeping ORDER BY stable and the flextime page-boundary comparison exact ✓

Applied consistently across EAP endpoints under SORT_SEMVER:

  • Trace item table (ORDER BY / GROUP BY) — string columns only.
  • Attribute names — orders names by their semver key (ClickHouse ORDER BY plus a Python re-sort that merges in synthetic non-stored keys), and honors descending.
  • Attribute values — orders values by semver key; string values only, so boolean-typed keys keep plain ordering.
  • Flextime pagination — page-boundary comparisons wrap both sides in the same semver key so follow-up pages stay consistent with the ORDER BY.

Dependency / merge

  • Merged latest master.
  • Bumped sentry-protos to 0.47.0, which provides the dedicated SORT_SEMVER OrderBy value (this replaces the earlier approach that overloaded SORT_NATURAL).

Changes

File Change
snuba/web/rpc/common/common.py semver_sort_key() helper + _SEMVER_COMPONENT_COUNT
snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py Apply the semver key in ORDER BY/GROUP BY for string columns under SORT_SEMVER
snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py Semver name ordering (CH + Python mirror _semver_sort_key_py), descending support, Unicode-digit-safe parsing
snuba/web/rpc/v1/trace_item_attribute_values.py Semver value ordering under SORT_SEMVER, guarded to string values
snuba/web/rpc/common/pagination.py Semver-consistent flextime page boundaries + page-token value-type validation
pyproject.toml / uv.lock Bump sentry-protos to 0.47.0
tests/… Unit tests for the sort-key shape and page-token validation; integration tests for table / attribute-names / attribute-values ordering (ASC, DESC, prerelease, build metadata, prefix, length normalisation)

Test plan

  • Unit: TestSemverSortKey (expression shape), TestFlexibleTimeWindowPageFilters (page-token value-type validation) — no DB required.
  • Integration (ClickHouse): TestSemverSorting (trace item table) and the semver tests in the attribute names / values suites assert numeric ordering, prerelease < stable, build-metadata handling, @-prefix stripping, length normalisation, and DESC = reverse of ASC.

Fixes lexicographic ordering of release strings by wrapping
`sentry.release` in a `tuple(arrayResize(arrayMap(...), 4), is_stable)`
sort key when used in ORDER BY / GROUP BY in EAP trace item table
queries. This ensures:

- 1.2.9 sorts before 1.2.10 (numeric, not lexicographic)
- 1.2.3-beta.1 sorts before 1.2.3 (prerelease before stable)
- 1.2.3-beta.1 sorts after 1.2.2 (prerelease of newer > older stable)
- 1.2 and 1.2.0 are equal (arrayResize pads to 4 components)
- package@version strips the prefix before comparison

Uses only ClickHouse functions available on Altinity 25.3/25.8
(no naturalSortKey which requires upstream 26.3+).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
@phacops
phacops requested review from a team as code owners June 25, 2026 03:07
Comment thread snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py Outdated
Comment thread snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py Outdated
claude and others added 2 commits June 25, 2026 03:23
Two fixes for the semver sort key implementation:

1. Replace f.tuple(..., alias=alias) with FunctionCall(alias, "tuple", ...)
   to fix mypy "Optional[str] not compatible with str" error on the alias
   argument.

2. Only apply semver_sort_key in the ORDER BY path, not GROUP BY.
   GROUP BY sentry.release must use the raw attribute expression so that
   the SELECT (also the raw string) is a function of the GROUP BY key and
   ClickHouse accepts the aggregation query. ORDER BY on a function of a
   GROUP BY key is valid in ClickHouse, so semver ordering still works.
   This also fixes the 500 error in Sentry's test_semver_build test where
   the spans events API auto-groups by release.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Comment thread snuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py Outdated
claude added 3 commits June 25, 2026 03:32
…rison

The flextime pagination filter was comparing the raw release string
lexicographically, while ORDER BY now uses the semver tuple key.  This
caused rows to be skipped or repeated on follow-up pages when
`sentry.release` was in the order clause (e.g. "1.2.3-beta.1" would be
missed because it sorts after "1.2.3" lexicographically but before it
semantically).

Fix: move `SEMVER_SORT_ATTRIBUTES` to common.py so pagination.py can
import it.  In `get_filters`, wrap both the column reference and the
stored literal value in `semver_sort_key(...)` for any semver attribute
so the page-boundary `<` comparison uses the same ordering as ORDER BY.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
sentry.release is coalesced from multiple attribute columns
(attributes_string_25['sentry.release'] + attributes_string_30['release']),
which makes the expression Nullable(String).  ClickHouse forbids
Nullable(Array(...)), so splitByChar on the raw expression would throw:
  Code: 43 – Nested type Array(String) cannot be inside Nullable type

Fix: apply ifNull(expr, '') at the start of semver_sort_key to convert
Nullable(String) to String before any string→array operations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
"1.2" and "1.2.0" normalise to the same sort key ([1,2,0,0], 1).
ClickHouse may return them in either relative order within the tied
pair, making the strict `asc == list(reversed(desc))` assertion
non-deterministic.  Collapse contiguous tied elements into a frozenset
before comparing so the test is order-insensitive within ties while
still verifying that every other element is in exact reverse order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
@phacops
phacops marked this pull request as draft June 25, 2026 23:18
claude and others added 4 commits June 26, 2026 20:46
Resolve conflict in resolver_trace_item_table.py:
- Keep master's updated _groupby_order_by_expression docstring (TYPE_ARRAY
  note) plus the semver paragraph.
- Keep master's inlined _convert_order_by structure and use_array_map_columns
  aggregation arg while preserving for_order_by=True so SEMVER_SORT_ATTRIBUTES
  still get the semver tuple sort key in ORDER BY.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Master's ruff config enables flake8-bugbear; B905 requires an explicit
strict= argument on zip(). column_names and column_values are built in
lockstep so strict=True is correct (they always have equal length).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Resolve conflict in test_endpoint_trace_item_table.py: both branches
appended independent test code at end of file. Keep both the
TestSemverSorting class and master's
test_uniq_with_default_value_double_casts_to_float64.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
@phacops
phacops marked this pull request as ready for review June 28, 2026 21:10
Comment thread snuba/web/rpc/common/common.py Outdated

_SEMVER_COMPONENT_COUNT = 4 # major.minor.patch.build

SEMVER_SORT_ATTRIBUTES: frozenset[str] = frozenset({"sentry.release"})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include sentry.sdk.version here as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just hardcoded for tests, I’m going to merge this instead and let you choose getsentry/sentry-protos#334

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted sentry.sdk.version in b989fe9SEMVER_SORT_ATTRIBUTES is back to just sentry.release. Good to merge as-is; the broader attribute selection can come through sentry-protos#334.


Generated by Claude Code

claude and others added 3 commits June 29, 2026 13:48
SDK versions follow the same semantic-versioning format as releases, so
add sentry.sdk.version to SEMVER_SORT_ATTRIBUTES.  It is a TYPE_STRING
attribute, so ORDER BY and flextime pagination pick up the semver tuple
key automatically.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Per maintainer direction on the PR: keep the semver-sort attribute list
hardcoded to sentry.release only for now.  Broader/configurable attribute
selection will be handled separately via sentry-protos.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Comment thread snuba/web/rpc/common/pagination.py
claude added 4 commits June 29, 2026 15:53
Flextime pagination compares sentry.release with semver_sort_key under a
strict `less` tuple predicate. Distinct strings sharing the same numeric
key + stability flag (e.g. "1.2" and "1.2.0") compared equal, so rows tied
with the page cursor but not on the previous page could be skipped on
follow-up pages (Cursor Bugbot, Medium).

Append the raw (non-null) release string as a third tuple element in
semver_sort_key. This gives distinct release strings a deterministic total
order and makes the page-boundary comparison exact. Both ORDER BY and
pagination call semver_sort_key, so they stay consistent automatically.

- test_common: assert the 3-element tuple shape (raw-string tiebreaker).
- test_endpoint_trace_item_table: ties are now deterministic, so
  test_desc_is_reverse_of_asc reverts to exact-reverse and
  test_length_normalisation asserts "1.2" immediately precedes "1.2.0".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Resolve conflict in test_endpoint_trace_item_table.py: contextual conflict
around the end of the file. Keep both the TestSemverSorting class and
test_uniq_with_default_value_double_casts_to_float64 (each defined once, no
duplication).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Master reverted the "Cast aggregation to Float64 in coalesce" change
(#7908), which deleted this regression test and the FunctionCall import
from the test module.  The merge incorrectly kept the test, leaving it
referencing an undefined FunctionCall (ruff F821 / mypy) and asserting
reverted behavior.  Drop it to match master.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Resolve conflict in tests/web/rpc/test_common.py: both branches appended
independent test classes at the end of the file. Keep both TestSemverSortKey
and master's TestAnyAttributeFilterOption.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
@phacops
phacops requested a review from alexjillard July 3, 2026 03:38
claude added 3 commits July 3, 2026 03:46
Update sentry-protos to 0.39.0, which adds the OrderBy `sort` field
(SORT_UNSPECIFIED/SORT_DEFAULT/SORT_NATURAL) to the attribute names and
values requests (sentry-protos#334), and honor it in the attribute values
endpoint.

- Add natural_sort_key() to common.py: a general natural-order key that
  tokenizes into digit / non-digit runs and zero-pads digit runs so a
  lexicographic comparison matches natural order (e.g. "1.2.9" before
  "1.2.10", "item2" before "item10"). Built from primitives since
  naturalSortKey() is unavailable on Altinity 25.3/25.8.
- trace_item_attribute_values: when order_by.sort == SORT_NATURAL, order the
  value column by natural_sort_key; unset/SORT_DEFAULT keeps the historical
  lexicographic order. count() remains the primary ordering.
- Unit tests for natural_sort_key structure and integration tests contrasting
  natural vs lexicographic value ordering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
The 3 version-valued items added to the autouse fixture defaulted to
sentry.is_segment=true, inflating test_boolean_case's count from 8 to 11.
Move them into the two natural-sort tests via a helper so they don't
perturb count-based assertions in sibling tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
SORT_NATURAL now selects the appropriate sort key per attribute: release
attributes (SEMVER_SORT_ATTRIBUTES, e.g. sentry.release) use the semver key
(prerelease before stable, "pkg@" prefix stripped), while every other
attribute uses the general natural-sort key (digit runs compared
numerically). This gives both a general natural sort and a release-aware
sort under the single SORT_NATURAL option the proto exposes today.

A dedicated proto value (e.g. SORT_RELEASE) would let clients pick semver
ordering explicitly for any attribute; that needs a sentry-protos change and
can be wired here once it lands.

Add an integration test asserting the semver ordering for sentry.release
values (prerelease-before-stable, prefix stripped).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
gen_item_message stamps a default sentry.release on every fixture item, so
that value shows up (with a higher count) alongside the releases the test
writes. Assert only the relative semver order of the written releases rather
than the full value list.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Comment thread snuba/web/rpc/common/common.py
is_stable previously only flagged SemVer '-' prereleases, so PEP 440
dot-style dev builds like "24.7.0.dev0+<sha>" (Sentry's own sentry.release
format) were classified stable and sorted after their GA release. Define
stable as a plain dotted-numeric version instead, so both "1.2.3-beta.1" and
"24.7.0.dev0+<sha>" sort before their matching stable release.

Add a table integration test asserting the dev build sorts before GA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Comment thread tests/web/rpc/test_common.py Outdated
claude added 2 commits July 3, 2026 17:35
The is_stable expression changed from equals(position(...)) to
match(version, ...) in 75edf1d; update the unit test's structural
assertion accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Update sentry-protos to 0.40.0, which adds the OrderBy `sort` field to the
trace-item-table request as well as the attribute names/values requests. Make
SORT_NATURAL the client-driven, semver-aware ordering everywhere and remove the
hardcoded attribute list.

- common: remove SEMVER_SORT_ATTRIBUTES and the general natural_sort_key;
  semver_sort_key is now the single implementation of SORT_NATURAL.
- table resolver: apply semver_sort_key in ORDER BY when a string column's
  OrderBy.sort == SORT_NATURAL (no attribute gating); GROUP BY keeps the raw
  expression.
- flextime pagination: mark SORT_NATURAL page-boundary columns and apply the
  semver key on both sides so pages stay consistent with ORDER BY.
- attribute values + names endpoints: order by the semver key when
  SORT_NATURAL is requested; the names endpoint mirrors the ordering in its
  Python merge/re-sort of synthetic keys.
- tests: drive semver via SORT_NATURAL, assert default (unset) stays
  lexicographic, and cover the names endpoint.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Comment thread snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py
claude and others added 3 commits July 3, 2026 18:21
str.isdigit() is True for characters like "²" that int() cannot parse,
which would crash the names endpoint under SORT_NATURAL. Require
c.isascii() too, matching ClickHouse toUInt32OrZero (ASCII decimal only,
else 0).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
The re-sort key callback used a bare `dict` param (missing type args) and an
`object` return, which mypy rejects for list.sort's key. Accept
Mapping[str, Any] (covariant, fits both the synthetic dicts and Row) and
return Any.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Comment thread tests/web/rpc/test_common.py Outdated
claude added 2 commits July 21, 2026 18:19
Resolve conflicts, update sentry-protos to the latest release (0.47.0),
and switch EAP semver-aware ordering from the overloaded SORT_NATURAL
OrderBy option to the dedicated SORT_SEMVER option now available in the
proto schema.

Conflict resolutions:
- pyproject.toml / uv.lock: take master's newer dependency floors and
  bump sentry-protos from 0.43.0 to 0.47.0 (regenerated the lock).
- trace_item_attribute_values.py: keep the semver value-ordering branch
  on top of master's add_existence_check_to_map_attribute_reads rename.
- test_endpoint_trace_item_table.py: keep both master's
  convert_results empty-array test and the branch's TestSemverSorting.

Renamed the internal semver plumbing to match the new enum name:
_order_by_natural -> _order_by_semver, _NATURAL_FILTER_PREFIX ->
_SEMVER_FILTER_PREFIX, is_natural -> is_semver, and the natural=/semver=
test helper flags.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xfgiye86dAPn9JNeHp4kAi
- test_length_normalisation: use a `<` ordering check instead of a strict
  adjacency (`idx_120 - idx_12 == 1`) check, per review.
- TestSemverSortKey: hoist the inlined `snuba.query.dsl.column` imports to
  the module-level import already present, using `column` directly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xfgiye86dAPn9JNeHp4kAi
@phacops
phacops force-pushed the claude/semver-sorting-eap-clickhouse-lpg3gs branch from d3e1171 to 25fe042 Compare July 21, 2026 18:26
if request.order_by.sort == TraceItemAttributeValuesRequest.OrderBy.SORT_SEMVER:
value_order_expression: Expression = semver_sort_key(column("attr_value"))
else:
value_order_expression = column("attr_value")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semver sort lacks string guard

Medium Severity

SORT_SEMVER always wraps attr_value in semver_sort_key, with no TYPE_STRING check. This endpoint also accepts boolean keys, and semver_sort_key applies string functions like splitByChar and ifNull(..., ''), so boolean value queries with SORT_SEMVER can fail at ClickHouse. The table resolver already guards on string type for the same reason.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d3e1171. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4c74cfcSORT_SEMVER now only wraps the value in semver_sort_key when request.key.type == TYPE_STRING, mirroring the table resolver's guard. Boolean keys keep plain ordering. Added a regression test (test_boolean_case_with_semver_sort_does_not_crash).


Generated by Claude Code

Comment thread snuba/web/rpc/common/common.py Outdated
# PEP 440 dot-style dev/pre builds ("24.7.0.dev0+<sha>", the common form on
# sentry.release) — the latter has no '-', so a bare '-' check would wrongly
# tag it stable and sort the dev build after its GA release.
is_stable = f.match(version_no_prefix, literal(r"^[0-9]+(\.[0-9]+)*$"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus metadata breaks version parse

Low Severity

semver_sort_key strips only on - before splitting numeric components, so a + build/local suffix stays attached to the last segment. toUInt32OrZero then zeroes that component, and match marks the release unstable, so forms like 1.2.3+build sort as a prerelease of 1.2.0 instead of aligning with 1.2.3.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d3e1171. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4c74cfcsemver_sort_key (and its Python mirror _semver_sort_key_py) now strip +build metadata before splitting components and before the stability match, so 1.2.3+build sorts as stable 1.2.3 instead of a prerelease of 1.2.0. Added a table integration test (test_build_metadata_ignored).


Generated by Claude Code

Comment thread snuba/web/rpc/common/pagination.py
Address review findings on the SEMVER sort work:

- semver_sort_key: strip SemVer build metadata ("1.2.3+build") before
  parsing. Left attached it zeroed the last numeric component and failed
  the stability match, so "1.2.3+build" sorted as a prerelease of 1.2.0
  instead of as stable 1.2.3. Applied in both the ClickHouse expression and
  its Python mirror (_semver_sort_key_py).
- trace_item_attribute_values: only apply the semver key under SORT_SEMVER
  for TYPE_STRING values. This endpoint also enumerates boolean values, and
  the string-only semver functions would fail at ClickHouse; mirrors the
  string-type guard already in the table resolver.
- pagination.get_filters: a client-supplied page token with an unsupported
  timestamp value type desynced the parallel column lists and crashed the
  strict zip() with an opaque error. Resolve the value first and raise a
  clear error, mirroring the validation in create().

Added regression tests for each.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xfgiye86dAPn9JNeHp4kAi

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4c74cfc. Configure here.

Comment thread snuba/web/rpc/v1/endpoint_trace_item_attribute_names.py Outdated
SORT_SEMVER selects name ordering with the OrderBy column typically left
unset, but _order_by_name_descending only recognized COLUMN_NAME, so a
SORT_SEMVER request with descending=True still sorted ascending in both the
ClickHouse ORDER BY and the Python merge re-sort. Recognize semver ordering
in _order_by_name_descending so descending flips it. Added a regression
test asserting the descending result is the reverse of ascending.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xfgiye86dAPn9JNeHp4kAi
@phacops phacops changed the title feat(eap): semver sort key for sentry.release ORDER BY in EAP feat(eap): semver-aware ordering via SORT_SEMVER across EAP endpoints Jul 21, 2026
claude added 2 commits July 21, 2026 19:53
…-update-deps-7d43rt

# Conflicts:
#	pyproject.toml
#	uv.lock
Trim the verbose docstrings/inline comments added for the semver work
(semver_sort_key, pagination boundary handling, attribute-name re-sort,
resolver ORDER BY guard, values ordering) down to their essential
rationale. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xfgiye86dAPn9JNeHp4kAi
@phacops
phacops merged commit 17ecfb5 into master Jul 21, 2026
70 checks passed
@phacops
phacops deleted the claude/semver-sorting-eap-clickhouse-lpg3gs branch July 21, 2026 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants